Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #241 evidence JSON format #242

Merged
merged 1 commit into from
Apr 29, 2020
Merged

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Apr 29, 2020

Removed new and to_amino_bytes methods.

@liamsi
Copy link
Member

liamsi commented Apr 29, 2020

Integration tests fail because of #233. Unrelated to this PR and can be ignored.

pub enum Evidence {
/// Duplicate vote evidence
#[serde(rename = "tendermint/DuplicateVoteEvidence")]
DuplicateVote(DuplicateVoteEvidence),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yihuang 👍

/// Duplicate vote evidence
#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct DuplicateVoteEvidence {
#[serde(rename = "PubKey")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to see that the JSON produces CamelCase instead of snake_case for these fields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue to track it tendermint/tendermint#4958 !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I didn’t know that this would count as a bug. Thought it’s just a minor inconsistency.

@liamsi liamsi merged commit 2f29d7a into informalsystems:master Apr 29, 2020
#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct DuplicateVoteEvidence {
#[serde(rename = "PubKey")]
pub_key: PublicKey,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noted that in the go versions there is no pubkey field (anymore?). We need to remove this for compatibility. Tracking here: #300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants